-
Notifications
You must be signed in to change notification settings - Fork 182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(feel): Clarify edge cases in FEEL expressions #4222
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @saig0 I left some small wording suggestions please have a look 👀
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/language-guide/feel-temporal-expressions.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/language-guide/feel-unary-tests.md
Outdated
Show resolved
Hide resolved
.../version-8.5/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
.../version-8.5/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
.../version-8.5/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.5/components/modeler/feel/language-guide/feel-temporal-expressions.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.5/components/modeler/feel/language-guide/feel-unary-tests.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @saig0 🚀
@saig0 I will review this for you 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm @saig0 - I've left a few changes/suggestions that need to be applied to the other versions as well, and a single typo, but happy to approve if you accept these?
docs/components/modeler/feel/language-guide/feel-temporal-expressions.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/language-guide/feel-temporal-expressions.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-string.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
docs/components/modeler/feel/builtin-functions/feel-built-in-functions-conversion.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm 👍
@mesellings thank you for your fast feedback and suggestions. 👍 |
Extend the description of the functions `date()`, `number()`, and `substring()` to clarify the behavior for certain edge cases.
Extend the description when a unary-tests expression is fulfilled. Make it explicit what an expression with the special variable `?` must evaluate to true.
Co-authored-by: Nicola Puppa <[email protected]>
Co-authored-by: Mark Sellings <[email protected]>
7034f0b
to
0c06f6c
Compare
@mesellings / @nicpuppa I need another approval. I rebased the branch to |
🧹 Preview environment for this PR has been torn down. |
Description
Extend the FEEL documentation to clarify edge cases, related to the following issues:
?
feel-scala#887substring()
fails if the length is greater than the given string feel-scala#801date()
doesn't returnnull
for invalid dates feel-scala#867The changes are relevant for all versions 8.2+.
When should this change go live?
hold
label or convert to draft PR)PR Checklist
/versioned_docs
directory./docs
directory (aka/next/
).